[libc++][math] Provide overloads for cv-unqualified floating point types for std::signbit
#106566
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Zingam
reviewed
Aug 29, 2024
libcxx/docs/Status/Cxx23Papers.csv
Outdated
| @@ -39,7 +39,7 @@ | |||
| "`P2401R0 <https://wg21.link/P2401R0>`__","Add a conditional ``noexcept`` specification to ``std::exchange``","2021-10 (Virtual)","|Complete|","14.0","" | |||
| "","","","","","" | |||
| "`P0323R12 <https://wg21.link/P0323R12>`__","``std::expected``","2022-02 (Virtual)","|Complete|","16.0","" | |||
| "`P0533R9 <https://wg21.link/P0533R9>`__","``constexpr`` for ``<cmath>`` and ``<cstdlib>``","2022-02 (Virtual)","|In Progress|","","``isfinite``, ``isinf``, ``isnan`` and ``isnormal`` are implemented" | |||
| "`P0533R9 <https://wg21.link/P0533R9>`__","``constexpr`` for ``<cmath>`` and ``<cstdlib>``","2022-02 (Virtual)","|In Progress|","","``isfinite``, ``isinf``, ``isnan``, ``isnormal`` and ``signbit`` are implemented" | |||
There was a problem hiding this comment.
I was not aware that we have GitHub issues for each paper/issue that are fully synchronized with the CSV status files and that we can encode custom notes in the CSV files via Github issues. Amazing. I will track the progress as subtask. Thanks for the friendly reminder!
There was a problem hiding this comment.
@robincaloudis Please let me know if you need help with creating the subtasks while you wait for commit access. You can ping me on Discord too (same account name).
There was a problem hiding this comment.
Thanks. I will do so.
robincaloudis
force-pushed
the
rc-signbit-overloads
branch
from
7041ead
to
8810607
Compare
By using `_LIBCPP_PREFERRED_OVERLOAD` we make sure that a given overload is a better match than an otherwise equally good function declaration. Why is there an equally good function declaration in the first place? Underlying the Windows SDK is the UCRT, the universal C runtime, which clang-cl makes use of. The UCRT should provide only C library headers, but does on top comes with overloads for all cv-unqualified floating point types (float, double, long double) for `std::signbit()` in https://github.com/microsoft/win32metadata/blob/e012b29924c53aa941fc010850b68331b0c3ea80/generation/WinSDK/RecompiledIdlHeaders/ucrt/corecrt_math.h#L309-L322. In a certain way, this can be seen as a deviation from the C standard. We need to work around it.
robincaloudis
force-pushed
the
rc-signbit-overloads
branch
from
8810607
to
528be9e
Compare
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Robin Caloudis (robincaloudis) ChangesWhyFollowing up on #105946, this patch provides the floating point overloads for What
Full diff: https://github.com/llvm/llvm-project/pull/106566.diff 2 Files Affected:
diff --git a/libcxx/include/__math/traits.h b/libcxx/include/__math/traits.h
index 3d4f14fc9cd552..f3b1f03110ab71 100644
--- a/libcxx/include/__math/traits.h
+++ b/libcxx/include/__math/traits.h
@@ -34,8 +34,30 @@ namespace __math {
# define _LIBCPP_SIGNBIT_CONSTEXPR
#endif
-template <class _A1, __enable_if_t<is_floating_point<_A1>::value, int> = 0>
-_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI bool signbit(_A1 __x) _NOEXCEPT {
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+ bool
+ signbit(float __x) _NOEXCEPT {
+ return __builtin_signbit(__x);
+}
+
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+ bool
+ signbit(double __x) _NOEXCEPT {
+ return __builtin_signbit(__x);
+}
+
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+ bool
+ signbit(long double __x) _NOEXCEPT {
return __builtin_signbit(__x);
}
diff --git a/libcxx/test/std/numerics/c.math/signbit.pass.cpp b/libcxx/test/std/numerics/c.math/signbit.pass.cpp
index c85033e363ce55..a8a566f7de6434 100644
--- a/libcxx/test/std/numerics/c.math/signbit.pass.cpp
+++ b/libcxx/test/std/numerics/c.math/signbit.pass.cpp
@@ -70,9 +70,22 @@ struct TestInt {
}
};
+template <typename T>
+struct ConvertibleTo {
+ operator T() const { return T(); }
+};
+
int main(int, char**) {
types::for_each(types::floating_point_types(), TestFloat());
types::for_each(types::integral_types(), TestInt());
+ // Make sure we can call `std::signbit` with convertible types. This checks
+ // whether overloads for all cv-unqualified floating-point types are working
+ // as expected.
+ {
+ assert(!std::signbit(ConvertibleTo<float>()));
+ assert(!std::signbit(ConvertibleTo<double>()));
+ assert(!std::signbit(ConvertibleTo<long double>()));
+ }
return 0;
}
|
philnik777
reviewed
Sep 7, 2024
robincaloudis
force-pushed
the
rc-signbit-overloads
branch
2 times, most recently
from
15e1518
to
aef854f
Compare
robincaloudis
force-pushed
the
rc-signbit-overloads
branch
from
aef854f
to
9c24e8d
Compare
robincaloudis
force-pushed
the
rc-signbit-overloads
branch
from
5527761
to
d7e4b9b
Compare
philnik777
approved these changes
Sep 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
Following up on #105946, this patch provides the floating point overloads for
std::signbitas defined by P0533R9.What
template<class = void>as the universal C runtime (UCRT) needed for Clang-Cl comes with overloads for all cv-unqualified floating point types (float, double, long double) forstd::signbit()by itself in the WinSDK. In a certain way, this can be seen as a deviation from the C standard. We need to work around it as the compilation would otherwise error out due to duplicated definitions.